chore: add AutoLayout constraints to make inline messages visible in UIKit app #718
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced May 17, 2024
Sample app builds 📱Below you will find the list of the latest versions of the sample apps. It's recommended to always download the latest builds of the sample apps to accurately test the pull request.
|
levibostian
force-pushed
the
spr/main/5ab2c403
branch
from
d783dc5 to
84e6c73
Compare
levibostian
force-pushed
the
spr/main/025c7bf5
branch
from
f506cd0 to
cc11137
Compare
levibostian
commented
May 17, 2024
There was a problem hiding this comment.
storyboard modified because I added inline Views to the screen.
I have been talking with Jason on how to QA test this feature. He didn't have specific requirements as far as what screen these Views go, so I added to Dashboard for convenience.
levibostian
commented
May 17, 2024
There was a problem hiding this comment.
I added 2 inline Views to the Dashboard screen. So we can test customers can use storyboards or use code to add inline Views to their app.
levibostian
commented
May 17, 2024
There was a problem hiding this comment.
Because queue manager stores data in-memory, I had to make it a singleton. All changes in this commit are to make it a singleton.
levibostian
commented
May 17, 2024
There was a problem hiding this comment.
The main logic of this PR - AutoLayout constraints so messages show up on screen.
levibostian
force-pushed
the
spr/main/025c7bf5
branch
from
cc11137 to
4a32afa
Compare
levibostian
force-pushed
the
spr/main/5ab2c403
branch
from
84e6c73 to
90ed722
Compare
levibostian
commented
May 20, 2024
| @@ -8,8 +8,11 @@ protocol GistInstance: AutoMockable { | |||
| } | |||
|
|
|||
| public class Gist: GistInstance, GistDelegate { | |||
| var messageQueueManager: MessageQueueManager = DIGraphShared.shared.messageQueueManager | |||
There was a problem hiding this comment.
Changed because MessageQueueManager is a singleton now. Use a property getter to get the shared singleton instance.
levibostian
commented
May 20, 2024
| @@ -15,8 +16,9 @@ protocol MessageQueueManager: AutoMockable { | |||
| } | |||
|
|
|||
| // sourcery: InjectRegisterShared = "MessageQueueManager" | |||
| // sourcery: InjectSingleton | |||
There was a problem hiding this comment.
This class has always been used as a singleton. The whole SDK would use Gist.messageQueue. To make the code testable, I switched to instead make the class a singleton in the digraph as we do for all other code in the SDK.
levibostian
commented
May 20, 2024
| @@ -3,7 +3,8 @@ import Foundation | |||
| import UIKit | |||
|
|
|||
| protocol MessageQueueManager: AutoMockable { | |||
| var interval: Double { get set } | |||
There was a problem hiding this comment.
Although I do prefer to use property getter/setters for an API, I found this API to be safer with having the class be a singleton.
Here is some sample code with what I mean:
// If you use a property getter/setter API:
let queueManager: MessageQueueManager = ... get singleton instance
queueManager.interval = 500
// 🤔 Did we update the shared singleton `inventory` property? Or did we update the `queueManager` local variable, only?
// if we change to getter/setter functions:
MessageQueueManager.singletonInstance.setInventory(500)
// This is much more clear that we are editing the singleton, not a locally scoped instance.
levibostian
force-pushed
the
spr/main/5ab2c403
branch
from
90ed722 to
27f8be8
Compare
levibostian
commented
May 20, 2024
There was a problem hiding this comment.
Added autolayout code to the View so we can now display the web content in the View.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature-inline-inapp #718 +/- ##
=======================================================
Coverage ? 58.54%
=======================================================
Files ? 142
Lines ? 3983
Branches ? 0
=======================================================
Hits ? 2332
Misses ? 1651
Partials ? 0 ☔ View full report in Codecov by Sentry. |
SDK binary size reports 📊SDK binary size of this PRSDK binary size diff report between this PR and the main branch |
levibostian
force-pushed
the
spr/main/025c7bf5
branch
2 times, most recently
from
ebabee3 to
4d63f69
Compare
levibostian
force-pushed
the
spr/main/5ab2c403
branch
from
27f8be8 to
e79e89c
Compare
Ahmed-Ali
requested changes
May 23, 2024
levibostian
force-pushed
the
spr/main/025c7bf5
branch
2 times, most recently
from
5458040 to
5581d89
Compare
levibostian
force-pushed
the
spr/main/5ab2c403
branch
3 times, most recently
from
183e553 to
aeaaabe
Compare
|
Video added to description of PR. |
Part of: https://linear.app/customerio/issue/MBL-310/create-a-uikit-uiview-that-displays-an-inline-in-app-message-sent-from Create a WebView to display inline in-app message and display in the Inline UIView. To do this, we can re-use a lot of the same logic that Modal messages use to create WebViews and display them. Testing: There are some automated tests added in this commit. Reviewer notes: This commit will not show an in-app message if you were to add it to a sample app. Autolayout constraints need to be added to have Views resize and that will come in a future commit. commit-id:025c7bf5
levibostian
force-pushed
the
spr/main/025c7bf5
branch
from
5581d89 to
f0c5d97
Compare
…UIKit app Part of: https://linear.app/customerio/issue/MBL-310/create-a-uikit-uiview-that-displays-an-inline-in-app-message-sent-from This commit implements UIKit AutoLayout constraints to the Inline View and the WebView. Adding these constraints make inline messages visible inside of UIKit apps! The Inline View will start at height 0 and then after the webview content is loaded, it will instantly change the height to display the full in-app message. Testing: All testing is done manually in sample apps. AutoLayout constraints change the visuals of Views so viewing how the View looks in an app is required to test. If you want to test this commit in a sample app build, follow these instructions: * Open the UIKit app on device. * Immediately after the app opens, send yourself a test in-app message with element ID "test". * Wait 10 seconds. In this time, the in-app SDK will fetch the test in-app message and then will display it inline on the screen. commit-id:5ab2c403
levibostian
force-pushed
the
spr/main/5ab2c403
branch
from
aeaaabe to
1c38f27
Compare
Ahmed-Ali
approved these changes
Jun 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of: https://linear.app/customerio/issue/MBL-310/create-a-uikit-uiview-that-displays-an-inline-in-app-message-sent-from
This commit implements UIKit AutoLayout constraints to the Inline View and the WebView. Adding these constraints make inline messages visible inside of UIKit apps! The Inline View will start at height 0 and then after the webview content is loaded, it will instantly change the height to display the full in-app message.
Testing:
All testing is done manually in sample apps. AutoLayout constraints change the visuals of Views so viewing how the View looks in an app is required to test.
If you want to test this commit in a sample app build, follow these instructions:
Demo video showing this testing:
When I began recording the video, I had already sent 2 messages from Fly to my test device. That way when the app opens and fetches messages, it would find these 2 messages. Then, the app displays those 2 messages in 2 separate inline Views on the screen. The 2 inline Views are expected to display all of the content with the correct height and width to match the web content's aspect ratio.
demo.mov
Stack: